Fix manylinux NuGet publish date and remove deprecated ubuntu packages#1845
Fix manylinux NuGet publish date and remove deprecated ubuntu packages#1845
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidates manylinux OpenCV artifact caching and validation, adds runtime layout and disk-usage logging, refactors NuGet publish steps to per-package push with result aggregation, removes Ubuntu-specific runtime NuGet projects, and updates docs to prefer linux-x64 runtime packages. (43 words) Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/docfx/articles/intro.md (1)
12-12:⚠️ Potential issue | 🟡 MinorInconsistency: Supported Platforms section still references Ubuntu versions.
Line 12 lists "Ubuntu 22.04, 24.04" but this PR removes the Ubuntu-specific packages in favor of generic
linux-x64bindings. Consider updating this line to reflect the change.📝 Suggested fix
-- **Linux**: Ubuntu 22.04, 24.04, ARM +- **Linux**: x64 (glibc 2.28+), ARM🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docfx/articles/intro.md` at line 12, Update the Supported Platforms text to remove Ubuntu-specific wording and reference the new generic Linux bindings: replace "Linux: Ubuntu 22.04, 24.04, ARM" with something like "Linux (linux-x64), ARM" or "Linux (linux-x64) and ARM" in the Supported Platforms / intro section so it reflects the switch from Ubuntu-specific packages to generic linux-x64 bindings.
🧹 Nitpick comments (2)
.github/workflows/manylinux.yml (2)
155-170: Consider clarifying step names for readability.The step at line 155 is named "Save OpenCV cache (full)" but performs validation, while line 165 "Save OpenCV cache (full) - upload" does the actual cache save. This split is functionally correct (validation failure prevents upload), but the naming could be clearer.
📝 Suggested rename for clarity
- - name: Save OpenCV cache (full) + - name: Validate OpenCV install (full) if: steps.opencv-cache.outputs.cache-hit != 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main' run: | config=$(find ${GITHUB_WORKSPACE}/opencv_artifacts -name 'OpenCVConfig.cmake' | head -1) ... shell: bash - - name: Save OpenCV cache (full) - upload + - name: Save OpenCV cache (full) if: steps.opencv-cache.outputs.cache-hit != 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/manylinux.yml around lines 155 - 170, Rename the two steps so their names reflect their intent: change the first step currently named "Save OpenCV cache (full)" (the validation block that checks for OpenCVConfig.cmake) to something like "Validate OpenCV artifacts (before cache save)" and keep or change the second step "Save OpenCV cache (full) - upload" to "Save OpenCV cache (full)" or "Upload OpenCV cache (full)"; update only the name fields in the YAML so the validation step clearly indicates it’s a check and the later step clearly indicates it performs the cache upload.
245-260: Same naming suggestion applies to slim build validation/upload split.For consistency with any changes made to the full build steps.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/manylinux.yml around lines 245 - 260, Rename the slim cache steps and keys to mirror the naming convention used for the full build steps so they stay consistent; update the step names "Save OpenCV cache (slim)" and "Save OpenCV cache (slim) - upload" and the cache key string "opencv-${{ env.OPENCV_VERSION }}-manylinux2_28-slim-${{ hashFiles('cmake/opencv_build_options_slim.cmake') }}" to match the same pattern used by the full build (e.g., replace or align the "-slim" suffix/label and any prefix differences), and ensure the if conditions and path (${{ github.workspace }}/opencv_artifacts_slim) remain correct after renaming.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/docfx/articles/intro.md`:
- Line 12: Update the Supported Platforms text to remove Ubuntu-specific wording
and reference the new generic Linux bindings: replace "Linux: Ubuntu 22.04,
24.04, ARM" with something like "Linux (linux-x64), ARM" or "Linux (linux-x64)
and ARM" in the Supported Platforms / intro section so it reflects the switch
from Ubuntu-specific packages to generic linux-x64 bindings.
---
Nitpick comments:
In @.github/workflows/manylinux.yml:
- Around line 155-170: Rename the two steps so their names reflect their intent:
change the first step currently named "Save OpenCV cache (full)" (the validation
block that checks for OpenCVConfig.cmake) to something like "Validate OpenCV
artifacts (before cache save)" and keep or change the second step "Save OpenCV
cache (full) - upload" to "Save OpenCV cache (full)" or "Upload OpenCV cache
(full)"; update only the name fields in the YAML so the validation step clearly
indicates it’s a check and the later step clearly indicates it performs the
cache upload.
- Around line 245-260: Rename the slim cache steps and keys to mirror the naming
convention used for the full build steps so they stay consistent; update the
step names "Save OpenCV cache (slim)" and "Save OpenCV cache (slim) - upload"
and the cache key string "opencv-${{ env.OPENCV_VERSION
}}-manylinux2_28-slim-${{ hashFiles('cmake/opencv_build_options_slim.cmake') }}"
to match the same pattern used by the full build (e.g., replace or align the
"-slim" suffix/label and any prefix differences), and ensure the if conditions
and path (${{ github.workspace }}/opencv_artifacts_slim) remain correct after
renaming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f55995d-1ac3-47be-a3b8-191659650ae5
📒 Files selected for processing (7)
.github/workflows/manylinux.yml.github/workflows/publish_nuget.ymldocs/docfx/articles/intro.mdnuget/OpenCvSharp4.official.runtime.ubuntu.22.04-x64.csprojnuget/OpenCvSharp4.official.runtime.ubuntu.22.04-x64.slim.csprojnuget/OpenCvSharp4.official.runtime.ubuntu.24.04-x64.csprojnuget/OpenCvSharp4.official.runtime.ubuntu.24.04-x64.slim.csproj
💤 Files with no reviewable changes (4)
- nuget/OpenCvSharp4.official.runtime.ubuntu.24.04-x64.csproj
- nuget/OpenCvSharp4.official.runtime.ubuntu.22.04-x64.csproj
- nuget/OpenCvSharp4.official.runtime.ubuntu.22.04-x64.slim.csproj
- nuget/OpenCvSharp4.official.runtime.ubuntu.24.04-x64.slim.csproj
OpenCvSharp4.official.runtime.linux-x64packages by downloading the pre-builtnuget-packages-manylinuxartifact inpublish_nuget.ymlinstead of repacking with the publish-time.soubuntu.*csproj files (superseded by manylinux)linux-x64packagesSummary by CodeRabbit
Documentation
Chores / Packaging
Reliability